Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Keyboard navigation #2671

Merged
merged 1 commit into from
Jun 2, 2024
Merged

Keyboard navigation #2671

merged 1 commit into from
Jun 2, 2024

Conversation

pbek
Copy link
Member

@pbek pbek commented May 24, 2024 •
edited
Loading

Implements parts of #2480 and serves as basis for other keyboard shortcuts.

WORK IN PROGRESS!

TODO

  • Integrate a way to use the keyboard to call functions
  • Use j/k to navigate feed items
    • Find a proper item if none was selected
    • Mark item as read, if needed
  • Add changelog

Optional

  • Automatic scrolling in feed item list
  • Highlight current feed item in feed item list

Checklist

@pbek pbek requested a review from SMillerDev May 24, 2024 18:07
@pbek pbek assigned pbek and unassigned pbek May 24, 2024
@pbek pbek changed the title add: use vue-shortkey Keyboard navigation May 24, 2024
@pbek
Copy link
Member Author

pbek commented May 24, 2024

@SMillerDev, I tried to use https://www.npmjs.com/package/vue-shortkey in https://github.com/nextcloud/news/pull/2671/files to use the keyboard, seems quite ok, see (1).

Is there a "current feed item" already, so I can try to find the previous and next item, see (2)?

image

@pbek
Copy link
Member Author

pbek commented May 24, 2024 •
edited
Loading

This is the current item: this.$store.getters.selected.
And those are all items in the list: this.$store.getters.allItems?

@pbek
Copy link
Member Author

pbek commented May 24, 2024

Jumping to the previous and next item kind of works, but I cannot use this.$store.getters.allItems directly, I need to be more aware in what view I am.
And it would be great to mark the current item in the list.

@pbek
Copy link
Member Author

pbek commented May 24, 2024 •
edited
Loading

Thought too complicated, I just need to use this.items. 🤦🏻

It's filterSortedItems() 😊

@pbek pbek force-pushed the feature/keyboard-shortcuts branch from 9d4d6c0 to 7c62718 Compare May 24, 2024 20:05
@pbek
Copy link
Member Author

pbek commented May 25, 2024

@Grotax, @SMillerDev, any feedback so far? Keyboard navigation with j/k works fine for me.

I wanted to scroll to the current item, but d21f1a7 didn't seem to do anything in the VirtualScroll. ️🤷🏻

And it would be great to introduce some different highlighting of the "active item" in the list. Haven't managed to do so.

Any suggestions on the both or feedback to the current code?

@Grotax
Copy link
Member

Grotax commented May 25, 2024

Didn't have time to look at it yet.

@pbek pbek removed the in progress label May 27, 2024
@pbek pbek requested a review from Grotax May 27, 2024 09:22
@pbek pbek marked this pull request as ready for review May 27, 2024 09:29
@pbek
Copy link
Member Author

pbek commented May 31, 2024

Anything I can help with?

@Grotax
Copy link
Member

Grotax commented Jun 1, 2024

Hi, looks good to me also tried it J/K works for me too. I have to say me and @SMillerDev have mostly only taken care of the backend. The frontend is kind of unmaintained. Which is why we hope to find someone that is willing to take care of the frontend long-term, to update it from time to time and slowly fix bugs.

What I would like to see before we merge:

While at it you could maybe also rebase on top of master the changelog is conflicting now :)

Thanks for working on this :)

@pbek pbek force-pushed the feature/keyboard-shortcuts branch from b723105 to 9bc3a18 Compare June 1, 2024 15:08
@pbek pbek closed this Jun 1, 2024
@pbek pbek force-pushed the feature/keyboard-shortcuts branch from 9bc3a18 to 9ff0f77 Compare June 1, 2024 15:09
@pbek
Copy link
Member Author

pbek commented Jun 1, 2024

I discarded the my squashed commit by accident in the GitHub interface, while I wanted to discard my commit in my master branch, that closed this PR and I can't reopen it 🙄

@pbek
Copy link
Member Author

pbek commented Jun 1, 2024

Hm, and now I can?

@pbek
add: j/k shortcuts
d58ee62 
add: make first attempt to jump to previous and next feed item

fix: linter errors

fix: use correct feed items

fix: use filterSortedItems()

add: trigger click event programmatically to benefit from item handling
inside FeedItemRow component

add: use a proper item if none is selected

fix: remove not needed MUTATIONS import

add: attempt to scroll to clicked item

doc: add changelog text

Signed-off-by: Patrizio Bekerle <patrizio@bekerle.com>
@pbek pbek reopened this Jun 1, 2024
@pbek
Copy link
Member Author

pbek commented Jun 1, 2024

Squashed, signed off and rebased to the current master...

@pbek
Copy link
Member Author

pbek commented Jun 1, 2024

The frontend is kind of unmaintained. Which is why we hope to find someone that is willing to take care of the frontend long-term, to update it from time to time and slowly fix bugs.

There is a big mountain of missing stuff... 🗻 😅
I usually have my hand full with https://github.com/pbek/QOwnNotes and all the surrounding tooling. 😆

@Grotax Grotax merged commit ea71ace into nextcloud:master Jun 2, 2024
22 of 40 checks passed
@pbek
Copy link
Member Author

pbek commented Jun 2, 2024

Thank you!

@pbek pbek deleted the feature/keyboard-shortcuts branch June 2, 2024 17:31
This was referenced Jun 2, 2024
Merged
Closed
Grotax added a commit that referenced this pull request Jun 10, 2024
@Grotax
Release 25.0.0-alpha7
9fbb6ad 
Changed
- added alternative development environment (#2670)
- Implement `j` and `k` keyboards shortcuts for navigating through feed items (#2671)
- Implement `s`, `i` and `l` keyboards shortcuts for staring current feed item (#2677)
- Implement `o` keyboards shortcut for opening the URL of current feed item (#2677)
- Implement `u` keyboards shortcut for marking current feed item read/unread (#2677)
- Implement highlighting of active feed item (#2677)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Jun 10, 2024
Merged
Grotax added a commit that referenced this pull request Jun 11, 2024
@Grotax
Release 25.0.0-alpha7
8688bf6 
Changed
- added alternative development environment (#2670)
- Implement `j` and `k` keyboards shortcuts for navigating through feed items (#2671)
- Implement `s`, `i` and `l` keyboards shortcuts for staring current feed item (#2677)
- Implement `o` keyboards shortcut for opening the URL of current feed item (#2677)
- Implement `u` keyboards shortcut for marking current feed item read/unread (#2677)
- Implement highlighting of active feed item (#2677)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@nioupola
Copy link

Hello,

Thank you for the work with keyboard. Please to use it.
I noticed that the console and inside Nextcloud an error is triggered if the feed is empty and try to use j/k.

TypeError: e is undefined

Uncaught (in promise) TypeError: e is undefined
    clickItem https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    jumpToNextItem https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    kn https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    n https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    _wrapper https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    m https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    keyDown https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2
    exports https://domain.tld/apps/news/js/nextcloud-news-main.js?v=1965c3ee-8:2

@pbek
Copy link
Member Author

pbek commented Jun 13, 2024

Thank you, I'll fix that.

@pbek pbek mentioned this pull request Jun 13, 2024
Merged
@pbek
Copy link
Member Author

pbek commented Jun 13, 2024

Should be fixed in #2689.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Grotax Grotax Grotax approved these changes

@SMillerDev SMillerDev Awaiting requested review from SMillerDev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pbek @Grotax @nioupola